Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate morph method #597

Merged
merged 2 commits into from
Jul 25, 2022
Merged

Consolidate morph method #597

merged 2 commits into from
Jul 25, 2022

Conversation

julianrubisch
Copy link
Contributor

Enhancement

Description

Apply the same logic to morph, even if it's a page morph

Why should this be added

Another small one from a family of upcoming patches (and inspired by a pairing session), this aims to consolidate the logic around page morphs a bit to the other morph types. There's more to come - I'd like to go full OO here, but this one should come first, and seems straightforward.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing
  • This is not a documentation update

Please note that the best way to suggest changes or updates to the documentation is to join Discord and leave a note in the #docs channel. Any documentation updates posted as PRs cannot be accepted at this time. ❤️

Copy link
Contributor

@leastbad leastbad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thanks for attacking this technical debt.

@julianrubisch
Copy link
Contributor Author

Before we get too excited, I just realized I need to give broadcast_halted etc the same treatment?

Copy link
Contributor

@leastbad leastbad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see what you mean, now.

It's a shame this added so much boilerplate... but I know that you have a larger plan. LGTM

@julianrubisch
Copy link
Contributor Author

I think it’s better to be explicit for the moment.

@julianrubisch julianrubisch merged commit 6b0a925 into master Jul 25, 2022
@julianrubisch julianrubisch deleted the consolidate-morph-method branch July 25, 2022 12:43
leastbad added a commit to leastbad/stimulus_reflex that referenced this pull request Sep 14, 2022
leastbad added a commit that referenced this pull request Sep 14, 2022
* created Stimulus.app

* elementToXPath supports html element

* reflexes Proxy

* dispatchLifecycleEvent converted to bind reflex

* top level bind doesn't work with rollup

* no need for finalStage concept anymore

* added queued and delivered stages to templates

* simplify lifecycle method signatures

* removed action cable specific events

* simplified callbacks module

* renaming reflex body param to error

* reworked client logging to operate on reflex params

* extract duration and progress from log functions

* pluggable transport mechanisms!

* changed mode to plugin

* changed forbidden reflexes to reject promise

* return error string to forbidden promise

* revert forbidden from reject to resolve

* tweaks to support non-isolation mode

* touch up comments in callback.js

* added TODO comment wrt resolveLate

* refactored reflex_store.js to reflexes.js, received() to process.js

* server side changes to deprecate reflex_id in favor of id

* client side modifications to rename reflexId to id

* localReflexControllers can import Stimulus

* revert #597 😞

* added lifecycle accessor to reflex instance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants